MULTIARCH-3965: Check if PER is enabled in the target PowerVS workspace by miyamotoh · Pull Request #7683 · openshift/installer

miyamotoh · 2023-11-06T15:44:12Z

Power Edge Router (aka. "PER") needs to be enabled in the target region to connect PowerVS to VPC, and thus is required for IPI to proceed. Hence this new validation check before doing anything substantial with Terraform.

Signed-off-by: Hiro Miyamoto miyamotoh@us.ibm.com

miyamotoh · 2023-11-06T22:27:16Z

/retest-required

mjturek · 2023-11-06T23:54:19Z

Closes MULTIARCH-3965

mjturek · 2023-11-06T23:56:40Z

/lgtm

hamzy · 2023-11-06T23:58:00Z

FYI and not a review, but fix the golint errors

miyamotoh · 2023-11-07T20:48:27Z

/retest-required

miyamotoh · 2023-11-07T23:08:50Z

/retest-required

miyamotoh · 2023-11-08T02:55:38Z

/retest-required

miyamotoh · 2023-11-08T13:35:57Z

/retest-required

pkg/asset/installconfig/powervs/client.go

miyamotoh · 2023-11-08T22:24:59Z

/retest-required

miyamotoh · 2023-11-09T01:29:53Z

/retest-required

hamzy · 2023-11-09T14:22:59Z

pkg/asset/installconfig/powervs/client.go

Also here? At least mention that it is in GetDatacenterCapabilities?

Addressed in the latest force-push. PTAL.

hamzy · 2023-11-09T14:25:52Z

pkg/asset/installconfig/powervs/validation.go

ServiceInstanceID is going away in my PR. Let's not introduce it again.

mjturek · 2023-11-09T14:38:08Z

/retest-required

miyamotoh · 2023-11-09T22:08:37Z

@r4f4 @mjturek could you please review and approve?

r4f4 · 2023-11-09T22:15:27Z

/retest-required

r4f4 · 2023-11-09T22:21:37Z

pkg/asset/installconfig/platformprovisioncheck.go

Doesn't need to be addressed in this PR, but maybe powervs can follow the pattern of the other platforms and use a ValidateForProvisioning function that encapsulates all the different validations needed.

r4f4 · 2023-11-09T22:28:11Z

pkg/asset/installconfig/powervs/validation.go

Could "power-edge-router" not show up in the set of capabilities? If so, maybe it's better to separate the error types (i.e. "capability not found" vs "capability not available")?

Addressed in the latest force-push. PTAL.

r4f4 · 2023-11-09T23:04:57Z

pkg/asset/installconfig/powervs/validation_test.go

For completeness, a test case where "power-edge-router" is not available.

Addressed in the latest force-push. PTAL.

r4f4 · 2023-11-09T23:08:00Z

Jira issues are now required in all PRs.

Signed-off-by: Hiro Miyamoto <miyamotoh@us.ibm.com>

openshift-ci-robot · 2023-11-10T00:40:28Z

@miyamotoh: This pull request references MULTIARCH-3965 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Power Edge Router (aka. "PER") needs to be enabled in the target region to connect PowerVS to VPC, and thus is required for IPI to proceed. Hence this new validation check before doing anything substantial with Terraform.

Signed-off-by: Hiro Miyamoto miyamotoh@us.ibm.com

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

r4f4

/approve

openshift-ci · 2023-11-10T00:46:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: r4f4

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [r4f4]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

r4f4

/lgtm

mjturek · 2023-11-10T16:22:27Z

/lgtm

mjturek · 2023-11-10T16:22:59Z

/retest-required

miyamotoh · 2023-11-10T17:21:55Z

/retest-required

openshift-ci-robot · 2023-11-10T18:10:33Z

/retest-required

Remaining retests: 0 against base HEAD f60ebb0 and 2 for PR HEAD 196d7d2 in total

miyamotoh · 2023-11-10T21:27:09Z

/retest-required

miyamotoh · 2023-11-11T14:43:53Z

/test openstack-manifests

mjturek · 2023-11-11T22:24:17Z

We cannot get openstack-manifests test to pass. Is there an outage with this test?

r4f4 · 2023-11-12T09:25:15Z

We cannot get openstack-manifests test to pass. Is there an outage with this test?

Yes. See the fix at #7617

openshift-ci-robot · 2023-11-12T10:28:20Z

/retest-required

Remaining retests: 0 against base HEAD 75bca37 and 1 for PR HEAD 196d7d2 in total

openshift-ci · 2023-11-12T12:56:02Z

@miyamotoh: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2023

openshift-ci bot requested review from barbacbd and pawanpinjarkar November 6, 2023 15:48

miyamotoh force-pushed the check-if-per-enabled branch 2 times, most recently from 106c9ab to 7b81ab5 Compare November 6, 2023 21:53

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2023

openshift-ci bot assigned mjturek Nov 6, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2023

miyamotoh force-pushed the check-if-per-enabled branch from 7b81ab5 to 47286fe Compare November 7, 2023 19:24

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2023

miyamotoh force-pushed the check-if-per-enabled branch from 47286fe to 78148ac Compare November 7, 2023 22:29

barbacbd reviewed Nov 8, 2023

View reviewed changes

pkg/asset/installconfig/powervs/client.go Outdated Show resolved Hide resolved

miyamotoh force-pushed the check-if-per-enabled branch 2 times, most recently from 815d742 to ab43837 Compare November 8, 2023 21:56

hamzy reviewed Nov 9, 2023

View reviewed changes

miyamotoh force-pushed the check-if-per-enabled branch 4 times, most recently from 9740bca to ff28556 Compare November 9, 2023 17:54

miyamotoh force-pushed the check-if-per-enabled branch from ff28556 to c9705ea Compare November 9, 2023 21:21

r4f4 reviewed Nov 9, 2023

View reviewed changes

miyamotoh force-pushed the check-if-per-enabled branch from c9705ea to e4287c9 Compare November 9, 2023 22:55

r4f4 reviewed Nov 9, 2023

View reviewed changes

Add ValidatePERAvailability & support logic/test

196d7d2

Signed-off-by: Hiro Miyamoto <miyamotoh@us.ibm.com>

miyamotoh force-pushed the check-if-per-enabled branch from e4287c9 to 196d7d2 Compare November 10, 2023 00:39

miyamotoh changed the title ~~Check if PER is enabled in the target PowerVS workspace~~ MULTIARCH-3965: Check if PER is enabled in the target PowerVS workspace Nov 10, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 10, 2023

r4f4 reviewed Nov 10, 2023

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2023

r4f4 reviewed Nov 10, 2023

View reviewed changes

openshift-ci bot assigned r4f4 Nov 10, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2023

openshift-merge-bot bot merged commit 0b368d7 into openshift:master Nov 12, 2023

miyamotoh mentioned this pull request Nov 16, 2023

OCPBUGS-23399: Check if PER is enabled in the target PowerVS workspace #7736

Merged

Comments

Conversation

miyamotoh commented Nov 6, 2023

Uh oh!

miyamotoh commented Nov 6, 2023

Uh oh!

mjturek commented Nov 6, 2023

Uh oh!

mjturek commented Nov 6, 2023

Uh oh!

hamzy commented Nov 6, 2023

Uh oh!

miyamotoh commented Nov 7, 2023

Uh oh!

miyamotoh commented Nov 7, 2023

Uh oh!

miyamotoh commented Nov 8, 2023

Uh oh!

miyamotoh commented Nov 8, 2023

Uh oh!

Uh oh!

miyamotoh commented Nov 8, 2023

Uh oh!

miyamotoh commented Nov 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjturek commented Nov 9, 2023

Uh oh!

miyamotoh commented Nov 9, 2023

Uh oh!

r4f4 commented Nov 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r4f4 commented Nov 9, 2023

Uh oh!

openshift-ci-robot commented Nov 10, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

r4f4 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Nov 10, 2023

Uh oh!

r4f4 left a comment

Choose a reason for hiding this comment

Uh oh!

mjturek commented Nov 10, 2023

Uh oh!

mjturek commented Nov 10, 2023

Uh oh!

miyamotoh commented Nov 10, 2023

Uh oh!

openshift-ci-robot commented Nov 10, 2023

Uh oh!

miyamotoh commented Nov 10, 2023

Uh oh!

miyamotoh commented Nov 11, 2023

Uh oh!

mjturek commented Nov 11, 2023

Uh oh!

r4f4 commented Nov 12, 2023

Uh oh!

openshift-ci-robot commented Nov 12, 2023

Uh oh!

openshift-ci bot commented Nov 12, 2023

Uh oh!

openshift-ci-robot commented Nov 10, 2023 •

edited by openshift-ci bot

Loading